Skip to content

Comments

Fix locating of version when tagging Docker images on Git tag push#1834

Merged
mre merged 1 commit intolycheeverse:masterfrom
eread:eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push
Oct 1, 2025
Merged

Fix locating of version when tagging Docker images on Git tag push#1834
mre merged 1 commit intolycheeverse:masterfrom
eread:eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push

Conversation

@eread
Copy link
Contributor

@eread eread commented Sep 1, 2025

To contribute to #1833

Fix locating of version when tagging Docker images on Git tag push and also remove tagging major version only, because it would resolve to 'version 0' at the moment.

type=semver,pattern={{major}}.{{minor}},value=${{ github.ref_name }},prefix=lychee-
type=semver,pattern={{major}},value=${{ github.ref_name }},prefix=lychee-
type=semver,pattern=lychee-{{version}}
type=semver,pattern=lychee-{{major}}.{{minor}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, pattern helps the action locate the version string itself to use to tag the Docker image. I think the fact that pattern is just currently the whole tag is why there are build warnings: https://github.com/lycheeverse/lychee/actions/runs/17205362504/job/48804656733#step:3:54.

I think prefix would add lychee- to the version that's found for tagging the Docker image - perhaps that's wanted so that it matches the tag. The last version tagged with a version number was simply tagged 0.15.0 (https://hub.docker.com/layers/lycheeverse/lychee/0.15.0/images/sha256-f71cb370f67fe81eccab4a8b0ff0539c2859f6fd1c80087078922eba07f9dace) and 0.15.0-alpine (https://hub.docker.com/layers/lycheeverse/lychee/0.15.0-alpine/images/sha256-18cf46741b0a92244c55670895cf7893c75f19f097f469c1274bf6b53c197ece).

@eread
Copy link
Contributor Author

eread commented Sep 1, 2025

@mre Could you review this one against the documentation for semver type: https://github.com/marketplace/actions/docker-metadata-action#typesemver?

I'm imaging that if, for example, this change was in place when the lychee-v0.20.1 tag was pushed:

  1. The type=semver lines would be triggered, because type=semver is used on push tag events.
  2. The pattern=lychee- settings would help pattern match just the version string, and would return proper semver.
  3. By having this action return something valid twice, there would be two additional --tag options added to the Docker image build command in a later action.

If correct, there would've been, in the end:

  • lycheeverse/lychee:0.20.1
  • lycheeverse/lychee:0.20
  • lycheeverse/lychee:0.20.1-alpine
  • lycheeverse/lychee:0.20-alpine

Does that make sense? Have I missed anything? Is that the tagging scheme you want, or do you want something different?

@mre
Copy link
Member

mre commented Sep 1, 2025

Didn't take a look yet, but just posting a comment that this would affect the docs at https://github.com/lycheeverse/lycheeverse.github.io as well. Maybe some other places, too? Can't think of any right now, but if someone depends on the Docker image tagging, then now is the time speak up. 😆

@eread
Copy link
Contributor Author

eread commented Sep 2, 2025

Didn't take a look yet, but just posting a comment that this would affect the docs at https://github.com/lycheeverse/lycheeverse.github.io as well. Maybe some other places, too? Can't think of any right now, but if someone depends on the Docker image tagging, then now is the time speak up. 😆

@mre Would this look dramatically different to what the project produced before the move to release-plz https://hub.docker.com/r/lycheeverse/lychee/tags?name=0.1?

@eread eread force-pushed the eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push branch from 3984fe1 to ac3e4c7 Compare September 2, 2025 01:31
@eread
Copy link
Contributor Author

eread commented Sep 2, 2025

@mre Based on the comment from @aravindan888 here: #1833 (comment), I've added back value=${{ github.ref_name }}. I interpret value as being a default value when the tag doesn't match anything. Not sure if we'll ever hit the logic for this, but it does make this a smaller change.

CC @trask

WDYT @aravindan888?

@eread eread force-pushed the eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push branch from ac3e4c7 to 745f46c Compare September 2, 2025 01:37
@eread
Copy link
Contributor Author

eread commented Sep 8, 2025

@mre Did you need me to do anything else on this one? Or is it just left open for comment?

@mre
Copy link
Member

mre commented Sep 25, 2025

Sorry, there's one comment here, which should be addressed before merging.
#1833 (comment)
Can you take a look?

@eread
Copy link
Contributor Author

eread commented Sep 26, 2025

Sorry, there's one comment here, which should be addressed before merging. #1833 (comment) Can you take a look?

Thanks for taking a look @mre. I think I addressed that, and mentioned in this comment: #1834 (comment). So I've put back value=${{ github.ref_name }} compared to an earlier version.

@eread eread force-pushed the eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push branch from 745f46c to 903d7bc Compare September 26, 2025 00:22
@eread
Copy link
Contributor Author

eread commented Sep 26, 2025

@mre I've also added another commit: 88c48a2 to fix an error with a job (https://github.com/lycheeverse/lychee/actions/runs/18024185042/job/51287994384) I got when I rebased this PR.

Not sure what changed, but explicitly specifying rustfmt is documented: https://github.com/dtolnay/rust-toolchain?tab=readme-ov-file#inputs.

Also remove tagging major version only, because it would resolve to
'version 0' at the moment.
@eread eread force-pushed the eread/fix-locating-of-version-when-tagging-docker-images-on-git-tag-push branch from 88c48a2 to 51a8259 Compare October 1, 2025 02:32
@eread
Copy link
Contributor Author

eread commented Oct 1, 2025

@mre I've also added another commit: 88c48a2 to fix an error with a job (https://github.com/lycheeverse/lychee/actions/runs/18024185042/job/51287994384) I got when I rebased this PR.

Not sure what changed, but explicitly specifying rustfmt is documented: https://github.com/dtolnay/rust-toolchain?tab=readme-ov-file#inputs.

@mre I've now removed that commit because it's been fixed upstream.

@mre
Copy link
Member

mre commented Oct 1, 2025

Looks fine to me. I don't yet know what implications it will have downstream, but we'll see about that. But from my side, this is worth merging to try and fix the Docker tags. Thanks for the work and for your patience, @eread.

@mre mre merged commit 54e425c into lycheeverse:master Oct 1, 2025
6 checks passed
@eread
Copy link
Contributor Author

eread commented Oct 8, 2025

Looks fine to me. I don't yet know what implications it will have downstream, but we'll see about that. But from my side, this is worth merging to try and fix the Docker tags. Thanks for the work and for your patience, @eread.

Thanks for the merge @mre.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants